Skip to content

lianad: warn createrecovery to an address of the same wallet. - #2226

Draft
qlrd wants to merge 1 commit into
wizardsardine:masterfrom
qlrd:feat/warn-own-address-recovery
Draft

lianad: warn createrecovery to an address of the same wallet.#2226
qlrd wants to merge 1 commit into
wizardsardine:masterfrom
qlrd:feat/warn-own-address-recovery

Conversation

@qlrd

@qlrd qlrd commented Jul 29, 2026

Copy link
Copy Markdown

This commit adds a new error (code 1001, just after the existing 1000) bound to createrecovery -- as a error/warning response -- when a user wants to re-lock funds from/to addresses derived from the wallet descriptor.

As stated by jp1ac4 the starting point is to check this on the backend since the backend has easier access to known wallet addresses (needs a check on how to deal with derivation indices beyond what the DB knows).

The proposed flow is first return an error from createrecovery, controlled by a new Option<bool> parameter allow_own_address (defaults to false). With this, a user can try again with allow_own_address=true (see tests/test_rpc.py).

In that case is proposed to a new warnings field was added to the response together with the psbt field, to warn the user as a mean to double-check the entire procedure.

refs #1654.

Details

A following commit to this PR is beign draft while waiting for discussion.

This commit adds a new error (code 1001, just after the existing 1000)
bound to `createrecovery` -- as a error/warning response -- when a user
wants to re-lock funds from/to addresses derived from the wallet
descriptor.

As stated by jp1ac4 the starting point is to check this on the backend
since the backend has easier access to known wallet addresses (needs a
check on how to deal with derivation indices beyond what the DB knows).

The proposed flow is first return an error from `createrecovery`,
controlled by a new `Option<bool>` parameter `allow_own_address`
(defaults to `false`). With this, a user can try again with
`allow_own_address=true` (see `tests/test_rpc.py`).

In that case is proposed to a new `warnings` field was added to the
response together with the `psbt` field, to warn the user as a mean to
double-check the entire procedure.

refs wizardsardine#1654.
Comment on lines +1332 to +1342
if sweep_addr_info.is_some() {
if !allow_own_address.unwrap_or(false) {
return Err(CommandError::RecoveryToOwnAddress(sweep_addr.addr));
}
warnings.push(format!(
"Address {} belongs to the same wallet. Recovered funds would \
be locked under the same descriptor again.",
sweep_addr.addr
));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better for maintenance

Suggested change
if sweep_addr_info.is_some() {
if !allow_own_address.unwrap_or(false) {
return Err(CommandError::RecoveryToOwnAddress(sweep_addr.addr));
}
warnings.push(format!(
"Address {} belongs to the same wallet. Recovered funds would \
be locked under the same descriptor again.",
sweep_addr.addr
));
}
if sweep_addr_info.is_some() {
let err = CommandError::RecoveryToOwnAddress(sweep_addr.addr.clone());
if !allow_own_address.unwrap_or(false) {
return Err(err.clone());
}
warnings.push(err.to_string());
}

coins_outpoints: &[bitcoin::OutPoint],
feerate_vb: u64,
timelock: Option<u16>,
allow_own_address: Option<bool>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can overlook something, but I dont think this flag is really useful, it seems to me we can just return warnings

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will get my annotations made before "that" and lyn :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants